-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update: Move template areas into a panel. #62033
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: -210 B (-0.01%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
b0c5399
to
d24098a
Compare
5f422e4
to
a75be74
Compare
a75be74
to
19d3e03
Compare
On this branch the Content panel seems to have disappeared when editing a page. There's also a small UX quirk; upon loading the Editor, if you click an area the tab switches from Page to Block. However on subsequent clicks this doesn't occur. This also seems to be a problem on trunk when editing a page, so perhaps one to fix separately. quirk.mp4Finally I think the icon should match the associated area. IE header template parts should have the |
Speaking of, there's been some changing behavior here. In the past, clicking a block would always switch to the block tab. In the site editor, the Page tab started to become a bit stickier, and now with the unification it feels like we have something in-between, where the page tab will be sticky if you click it, until you deselect all blocks (which is currently hard in the site editor) and then selecting a block again. What do you think should be the ideal behavior here? |
I don't know that there's a hard rule we can apply across, but in this particular case I find it slightly unexpected that clicking an item in the Content panel also switches tabs. |
That's a good point. But could that be the exception? That clicking any block in the canvas still selects the block tab (so the inspector can remain contextual by default), but clicking a content item doesn't? |
Yes, that might work. How about List View? I noticed the issue around inconsistent behavior exists there too. |
List view is a good question too, because an instance of it lives inside the navigation block for managing items there. I suspect that instance already has local code to not swap tabs when you're just selecting an item there. If that's the case, then I feel like selecting a block in the canvas should be the same as selecting a block in the list view. It's not because trunk works very poorly at the moment: it's mostly that the behavior is unpredictable. I don't know why sometimes the block tab is invoked, but not others. I think if it's mainly for the content panel, it would feel implied to not switch tabs when using those controls. |
Agreed. This feels like a good change for this PR, assuming it isn't a big can of worms 🤞 |
19d3e03
to
04b16eb
Compare
Hi @jasmussen, and @jameskoster, this PR was updated we now keep the template tab when an item is clicked. |
Regarding the other issues:
I'm not being able to reproduce this issue. @jameskoster would you be able to provide the steps you are following or a recording? Thank you in advance for the help.
Yes, this one seems like a bug on the list view things. I'm fixing this issue. |
This is now fixed. I think I addressed the pending items. It should be ready for a new look. |
No specific steps, just editing a page in the site editor.
The other issues appear to be fixed 👍 |
Hi @jameskoster, the issue of the page's content disappearing is fixing not sure why it did not happen probably tested on the wrong branch, but things should work now. Let me know if there is any other issue or if we can merge. Thank you for there reviews and testing 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be working now 🚀
{ renderingMode !== 'post-only' && ( | ||
<TemplateContentPanel /> | ||
) } | ||
<TemplateContentPanel renderingMode={ renderingMode } /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we passing renderingMode here, can't we just select it within the component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the unification of the content components.
* @param {Object} state Global application state. | ||
* @return {Array} Template parts and their blocks in an array. | ||
*/ | ||
export const getCurrentTemplateTemplateParts = createRegistrySelector( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This selector is still "aliased" in the site editor store and triggers an error now.
Should we also remove it?
gutenberg/packages/edit-site/src/store/selectors.js
Lines 241 to 247 in 387acef
export const getCurrentTemplateTemplateParts = createRegistrySelector( | |
( select ) => () => { | |
return unlock( | |
select( editorStore ) | |
).getCurrentTemplateTemplateParts(); | |
} | |
); |
Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
This PR removes the "Areas" section of templates and makes the content panel available for templates.
It also simplifies a lot the code as it uses BlockQuickNavigation which is generic for any block instead of something specific for template parts.
Part of the remaining items in #59689 (comment).
Screenshots or screencast
Before:
After: